Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve default security settings #439

Merged
merged 3 commits into from
Aug 11, 2023

Conversation

jawnsy
Copy link
Contributor

@jawnsy jawnsy commented Jul 28, 2023

  • Add default seccomp profile
  • Add pod security context
  • Enable read-only root filesystem by default
  • Disable default service links

Closes: #435

* Add default seccomp profile
* Add pod security context
* Enable read-only root filesystem by default
* Disable default service links

Closes: pganalyze#435
@jawnsy
Copy link
Contributor Author

jawnsy commented Jul 28, 2023

Tested this with the command:

helm upgrade pganalyze ./contrib/helm/pganalyze-collector/ --install --set=resources.requests.cpu=0 --set=resources.requests.memory=0

Here's the resulting pod (with a few fields removed):

$ k get pod pganalyze-pganalyze-collector-77bcd6b6b6-vgp4z -o yaml
apiVersion: v1
kind: Pod
metadata:
  creationTimestamp: "2023-07-28T03:44:56Z"
  generateName: pganalyze-pganalyze-collector-77bcd6b6b6-
  labels:
    app.kubernetes.io/instance: pganalyze
    app.kubernetes.io/name: pganalyze-collector
    pod-template-hash: 77bcd6b6b6
  name: pganalyze-pganalyze-collector-77bcd6b6b6-vgp4z
  namespace: jawnsy-test
  ownerReferences:
  - apiVersion: apps/v1
    blockOwnerDeletion: true
    controller: true
    kind: ReplicaSet
    name: pganalyze-pganalyze-collector-77bcd6b6b6
    uid: 9f8df1df-a047-48ce-ad07-b9e810af6faa
  resourceVersion: "70758548"
  uid: 8f834c68-6cdb-41f9-b1e3-00776df3ba46
spec:
  containers:
  - image: quay.io/pganalyze/collector:v0.50.1
    imagePullPolicy: IfNotPresent
    name: pganalyze-collector
    resources:
      limits:
        cpu: "1"
        memory: 1Gi
      requests:
        cpu: "0"
        memory: "0"
    securityContext:
      allowPrivilegeEscalation: false
      capabilities:
        drop:
        - ALL
      readOnlyRootFilesystem: true
      runAsGroup: 1000
      runAsNonRoot: true
      runAsUser: 1000
      seccompProfile:
        type: RuntimeDefault
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /tmp
      name: scratch
      subPath: tmp
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kube-api-access-w5ckd
      readOnly: true
  dnsPolicy: ClusterFirst
  enableServiceLinks: false
  nodeName: gke-internal-tools-d-application-13b4-a1815e79-tzn5
  preemptionPolicy: PreemptLowerPriority
  priority: 0
  restartPolicy: Always
  schedulerName: default-scheduler
  securityContext:
    runAsGroup: 1000
    runAsNonRoot: true
    runAsUser: 1000
    seccompProfile:
      type: RuntimeDefault
  serviceAccount: pganalyze-pganalyze-collector
  serviceAccountName: pganalyze-pganalyze-collector
  terminationGracePeriodSeconds: 30
  tolerations:
  - effect: NoExecute
    key: node.kubernetes.io/not-ready
    operator: Exists
    tolerationSeconds: 300
  - effect: NoExecute
    key: node.kubernetes.io/unreachable
    operator: Exists
    tolerationSeconds: 300
  volumes:
  - emptyDir: {}
    name: scratch
  - name: kube-api-access-w5ckd
    projected:
      defaultMode: 420
      sources:
      - serviceAccountToken:
          expirationSeconds: 3607
          path: token
      - configMap:
          items:
          - key: ca.crt
            path: ca.crt
          name: kube-root-ca.crt
      - downwardAPI:
          items:
          - fieldRef:
              apiVersion: v1
              fieldPath: metadata.namespace
            path: namespace
status:
  conditions:
  - lastProbeTime: null
    lastTransitionTime: "2023-07-28T03:44:56Z"
    status: "True"
    type: Initialized
  - lastProbeTime: null
    lastTransitionTime: "2023-07-28T03:44:57Z"
    status: "True"
    type: Ready
  - lastProbeTime: null
    lastTransitionTime: "2023-07-28T03:44:57Z"
    status: "True"
    type: ContainersReady
  - lastProbeTime: null
    lastTransitionTime: "2023-07-28T03:44:56Z"
    status: "True"
    type: PodScheduled
  containerStatuses:
  - containerID: containerd://10106bd5210591d1e464b6a9d316195e5719dcb0b8ded8e4a817ef93f7f5dcd9
    image: quay.io/pganalyze/collector:v0.50.1
    imageID: quay.io/pganalyze/collector@sha256:bdf21b3a1e9252fc9e6b8bda20a7089db77f79e677811e9592c1c8abaaf3a151
    lastState: {}
    name: pganalyze-collector
    ready: true
    restartCount: 0
    started: true
    state:
      running:
        startedAt: "2023-07-28T03:44:57Z"
  phase: Running
  qosClass: Burstable
  startTime: "2023-07-28T03:44:56Z"

volumeMounts:
- mountPath: /tmp
name: scratch
subPath: tmp
Copy link
Member

@lfittl lfittl Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the read-only root filesystem setting, I wonder if we also need to create a volume mount for /state?

(which is where the state file is written to - whilst that is mainly intended to support restarts outside of a container-based environment, it avoids errors that would likely occur when /state is no writeable)

See https://github.com/pganalyze/collector/blob/main/contrib/docker-entrypoint.sh#L23

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, absolutely! I didn't know about this file

@lfittl lfittl merged commit 9aad9bb into pganalyze:main Aug 11, 2023
6 checks passed
@lfittl
Copy link
Member

lfittl commented Aug 11, 2023

Merged with small edits - thanks for the contribution!

@jawnsy
Copy link
Contributor Author

jawnsy commented Aug 15, 2023

@lfittl Thanks for finishing this up and for merging! Apologies for the delay, I was out of town last week :)

@jawnsy jawnsy deleted the jawnsy/security-constraints branch August 15, 2023 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Additional security settings for pganalyze collector
2 participants